Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable/Disable Player Aging #18

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Conversation

gtabot
Copy link
Contributor

@gtabot gtabot commented Mar 17, 2021

  • Added a new parameter which enables/disables aging
  • It will hide/display several aging lines on the player depending on the player's age

@gtabot
Copy link
Contributor Author

gtabot commented Mar 17, 2021

This branch is built on top of the previous Editor branch and reflects the new editor changes

@gtabot gtabot mentioned this pull request Mar 17, 2021
@dumbmatter dumbmatter changed the base branch from master to foo March 18, 2021 01:39
@dumbmatter dumbmatter changed the base branch from foo to master March 18, 2021 01:39
@dumbmatter
Copy link
Member

^ branch switching was to get GitHub to update the diff in this PR after I merged the other PR, crazy that is still necessary!

Copy link
Member

@dumbmatter dumbmatter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This looks like it's on the right path, I just have a few ideas...

@@ -98,11 +98,30 @@ type FeatureInfo = {
};

const drawFeature = (svg: SVGSVGElement, face: Face, info: FeatureInfo) => {
const feature = face[info.name];
const feature = Object.assign({}, face[info.name]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? I think it doesn't do anything, but I may be missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change creates a copy of face[info.name] instead of passing the reference, this way face doesn't get changed even if the actual features drawn vary. The reason why I'm doing this will make sense after reading below.

My thinking and overall strategy for implementing the aging feature was that the parts and their values would act as master DNA for the person if aging.enabled = true. Another way to think of this is you are supplying what features the person can and will ultimately get as he ages. So for example, if the generated face has forehead lines, those lines won't be drawn until the person is in his older 30s. This way, the same JSON face object can go through the aging progression simply by incrementing the age value, rather than having to keep track of adding and subtracting parts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with that is you'd then have to generate every new face with a bunch of wrinkles and other old people features. Otherwise you'd have some that never age.

Ultimately you can get the same result either way, I just think it'd be more intuitive to have the base face object be the young version, and then add on stuff to represent aging. Because that's what actually happens as real humans age :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I'm more considering generating new faces rather than aging existing faces. I figured new face objects would have to be generated anyway to include the aging key/values.

However, I think by making the base face object the young version, it won't allow users to specify what aging lines will eventually occur when the face ages. Or if the deterministic aging isn't varied enough, you might see too many common combinations of heads/lines. I'd rather have the option to choose what my face will look like at 35 and then have the younger versions of the face be determined, especially since its basically just removal of lines. I do realize that's now how it works in real life but I believe this way allows for full customization, i.e. what if I want a different aging line for a face then what the deterministic random would apply.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way of phrasing that is... you pick all the facial features you want for your guy. Some will appear/disappear based on age. If you say he has a bunch of wrinkles, he won't when he's young. If you say he has long hair, he might not when he's old. I think that makes enough sense for people to understand.

src/display.ts Outdated
face.aging &&
face.aging.enabled &&
info.name === "hair" &&
feature.id === "short-bald" &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This handles one hair style, but not any of the others. Maybe in generate.ts, short-bald and bald should never be selected, and then here they can be activated at some age cutoff for other hair styles.

Fun way of doing that and having it feel random - somehow pick a deterministic random number seeded with some of the info in the face object. Use that to determine the age balding starts (short-bald) and ends (bald).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea. I'll make this change in the next commit!

@@ -311,6 +331,28 @@ const display = (
];

for (const info of featureInfos) {
if (face.aging && face.aging.enabled) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to what I wrote about hair... maybe switch this around, so some facial lines are just never (or more rarely) included in the base face object, and then apply them here (maybe again with deterministic randomness). I think that would make it easier to have aging appear in all faces. Since currently, when I'm playing around with this, a lot of faces are not impacted by aging.

Copy link
Contributor Author

@gtabot gtabot Mar 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Responding to the last part of your comment. My understanding is that you implemented this version in BBGM and you didn't see many aged faces? If so, first, were you able to inject the age value in aging.age?

If so, my thoughts are the way aging is currently set up (to use the parts as master DNA) a lot of your currently generated face models would not show the effects of aging. This is because I assume the proportion of faces that have aging lines are low combined with the fact that of these faces, only some are actually old enough to show effects of aging. Using this version of faces.js, most if not all generated faces should be randomly assigned aging lines (unless they're blessed with great genes) and those lines would appear appropriately as they age.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I very arbitrarily chose different ages for different lines to appear. They could easily be too high. There could also be a variable called maturity that can span +/- 3 or something which can add an element of variation across the players on when they reach these arbitrary age thresholds.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that you implemented this version in BBGM and you didn't see many aged faces?

No, just playing around in the editor UI. But it's the same thing, BBGM generates random faces the same way the editor does.

Also, I very arbitrarily chose different ages for different lines to appear. They could easily be too high. There could also be a variable called maturity that can span +/- 3 or something which can add an element of variation across the players on when they reach these arbitrary age thresholds.

Probably a good idea. On second thought, what I wrote before about using deterministic random numbers to determine aging might be kind of annoying, because then users wouldn't be able to control it...

lol complicated stuff the more you think about it!

@gtabot
Copy link
Contributor Author

gtabot commented Mar 22, 2021

I made some changes that better respect the aging.enabled parameter. When aging is enabled, aging lines are guaranteed to be selected when generate() is called. Also, aging is enabled by default.

@gtabot
Copy link
Contributor Author

gtabot commented Mar 22, 2021

Next, I will see what I can do about balding

@gtabot
Copy link
Contributor Author

gtabot commented Mar 25, 2021

Try checking out this version of the branch. What I like to do is click randomize in the editor then click on age and go all the way up/down with the arrow buttons to see that player's aging process.

</div>
<div class="col-5 form-group">
<input
type="number"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this to type="range" and you get a nifty slider. Should display the age as text next to it too.

</div>
<div class="row">
<div class="col-5">
<label for="aging-maturity">Maturity</label>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like having a "maturity" variable but I think it needs to be renamed, at least in the UI, to help people understand what it does. Maybe rename to "Aging speed" or something?

@dumbmatter
Copy link
Member

Overall this works pretty well! Fun stuff. I will probably merge it, maybe play around with it a bit, and release it... hopefully this week.

@TravisJB89 @domini7 just tagging you in case you guys want to take a look at this branch. Any feedback is welcome!

@gtabot
Copy link
Contributor Author

gtabot commented Mar 31, 2021

Awesome! @dumbmatter feel free to make those changes (or any other)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants